Skip to content

mi: fix missing braces around initializer warnings#1011

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:mi-missing-braces
May 19, 2025
Merged

mi: fix missing braces around initializer warnings#1011
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:mi-missing-braces

Conversation

@ikegami-t
Copy link
Copy Markdown
Contributor

The fix required for the muon build on CentOS.

@ikegami-t ikegami-t force-pushed the mi-missing-braces branch from b86c618 to 67fcef4 Compare May 12, 2025 23:10
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 13, 2025

Which version of CentOS and gcc is this? Is this really CentOS or CentOS stream? I thought CentOS is EOL.

@jk-ozlabs

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

Can you include the warnings in your commits or PR? this will help to verify the fix.

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

Based on the changes proposed here, it would be good to intentionally decide whether we want to support compilers that do not allow struct initializers - or is this just a braces thing?

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 13, 2025

This is also my thinking. If these are EOL distros I don't think we should support those old compilers.

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Yes the build warning errors caused on CentOS EOL. Yes now I am checking this on CentOS stream so will update later.

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Actually the warning errors below and attached build log on CentOS 7 EOL not happend on the CentOS 9 stream but is it really not supported the CentOS 7 EOL by the muon build since currently the CentOS 7 build described by the wiki page: https://github.com/linux-nvme/nvme-cli/wiki/Building-nvme‐cli?

[42/164] compiling c nvme.p/plugins/ocp/ocp-print-binary.c.o
In file included from ../subprojects/libnvme/src/libnvme-mi.h:17:0,
                 from ../subprojects/libnvme/examples/mi-mctp-ae.c:20:
../subprojects/libnvme/src/nvme/mi.h: In function 'nvme_mi_aem_ack':
../subprojects/libnvme/src/nvme/mi.h:1329:9: warning: missing braces around initializer [-Wmissing-braces]
  struct nvme_mi_aem_enable_list list = {0};
         ^
../subprojects/libnvme/src/nvme/mi.h:1329:9: warning: (near initialization for 'list.hdr') [-Wmissing-braces]

muon-build.txt

@ikegami-t
Copy link
Copy Markdown
Contributor Author

By the way the following error also still caused on the CentOS stream 9 actually so I will do investigate more later.

[142/166] linking subprojects/libnvme/test/test-mi
samu: job failed with status 1: gcc  -o subprojects/libnvme/examples/mi-mctp-csi-test subprojects/libnvme/examples/mi-mctp-csi-test.p/mi-mctp-csi-test.c.o -Wl,--as-needed -Wl,--no-undefined -static -Wl,--start-group subprojects/libnvme/src/libnvme-mi.a subprojects/libnvme/ccan/libccan.a -Wl,--end-group
subprojects/libnvme/examples/mi-mctp-csi-test.p/mi-mctp-csi-test.c.o: In function `do_csi_test':
/home/tokunori/nvme-cli/.build-ci/../subprojects/libnvme/examples/mi-mctp-csi-test.c:151: undefined reference to `pthread_create'
/home/tokunori/nvme-cli/.build-ci/../subprojects/libnvme/examples/mi-mctp-csi-test.c:162: undefined reference to `pthread_join'
collect2: error: ld returned 1 exit status
../plugins/wdc/wdc-nvme.c: In function 'wdc_print_unsupported_reqs_log_json':
../plugins/wdc/wdc-nvme.c:5381:71: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
   sprintf((char *)unsup_req_list_str, "Unsupported Requirement List %d", j);
                                                                       ^
../plugins/wdc/wdc-nvme.c:5381:3: note: 'sprintf' output between 31 and 41 bytes into a destination of size 40
   sprintf((char *)unsup_req_list_str, "Unsupported Requirement List %d", j);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
samu: subcommand failed

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

subprojects/libnvme/examples/mi-mctp-csi-test.p/mi-mctp-csi-test.c.o: In function `do_csi_test':
/home/tokunori/nvme-cli/.build-ci/../subprojects/libnvme/examples/mi-mctp-csi-test.c:151: undefined reference to `pthread_create'
/home/tokunori/nvme-cli/.build-ci/../subprojects/libnvme/examples/mi-mctp-csi-test.c:162: undefined reference to `pthread_join'
collect2: error: ld returned 1 exit status

ok, the new CSI test likely needs an explicit -lpthread dependency. @chorkin , can you check that out? (this is somewhat unrelated to the title of the issue though!)

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 14, 2025

For the CentOS 7, 8 case, I don't think we need to bother anymore. If there are still users out there using an EOL distro, they are own their own. That means we should just update our wiki.

CentOS stream: we could setup a CI build for this as well to catch these errors earlier.

-lpthread is kind of special in the world of meson. It is supposed to be added by the meson automatically. I struggled with this as well but can't remember what I did. Let me see.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 14, 2025

@ikegami-t
Copy link
Copy Markdown
Contributor Author

ikegami-t commented May 14, 2025

Okay I will do update the wiki as the CentOS 7 and 8 EOL dropped as unsupported later. By the way about the pthread issue I just thought as how about is to disable the test build using pthread on muon to avoid the muon build error? Thank you.

(Add)
Just update the wiki page: https://github.com/linux-nvme/nvme-cli/wiki/Building-nvme‐cli#centos-stream-9-just-copied-the-installation-from-centos-8.

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

There it is:

https://lore.kernel.org/linux-trace-devel/[email protected]/

nice!

so does that mean we're not supporting that old meson version in? or that we'll want to add the explicit pthread dependency?

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 15, 2025

I think meson would reject to build if the minimal version requirment is not met. Thus I think we support the old version of meson which doesn't add pthread. Let's add it explicitly and add a comment that when we update the minimal meson version requirement to check if this is still needed.

@ikegami-t ikegami-t force-pushed the mi-missing-braces branch 2 times, most recently from 0f08b98 to 16544a9 Compare May 16, 2025 16:20
@ikegami-t
Copy link
Copy Markdown
Contributor Author

Deleted the original commit and added the the pthread dependency commit instead. Thank you.

Older version of meson do not add automatically the pthread
dependency. Thus add it explicitly to thebuild.

Signed-off-by: Tokunori Ikegami <[email protected]>
@igaw igaw force-pushed the mi-missing-braces branch from 16544a9 to beb8bcb Compare May 19, 2025 11:59
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 19, 2025

rebased to resolve merge conflict.

@igaw igaw merged commit 7f000b6 into linux-nvme:master May 19, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants